Fix DurationFormatUtils.formatPeriod() calculation when pattern omits 'M' - #1780

@Alwaysgaurav1
You didn't follow instructions in the PR template, therefore this PR breaks the build. Don't push until the build passes locally.

@garydgregory Apologies for the oversight. I've fixed the Checkstyle issues and verified the build passes locally with mvn test checkstyle:check (0 Checkstyle violations, 47/47 tests passing). Thanks for the reminder!

There is one case this PR breaks that I just added to demonstrate the issue:

    @Test
    void testFormatPeriodWithoutMonthsAfterLeapDayAnniversary() {
        final TimeZone timeZone = TimeZone.getTimeZone("UTC");
        final Calendar start = Calendar.getInstance(timeZone);
        start.clear();
        // 2020 was not a leap year
        start.set(2020, Calendar.FEBRUARY, 29);
        final Calendar end = Calendar.getInstance(timeZone);
        end.clear();
        // 2021 was not a leap year
        end.set(2021, Calendar.MARCH, 1);
        assertEquals("1 years 1 days", DurationFormatUtils.formatPeriod(start.getTimeInMillis(), end.getTimeInMillis(), "y' years 'd' days'", false, timeZone));
    }

There is no standard for normalizing dates in non-leap years.
The main branch code normalizes Feb 29 2020 to Feb 28 2020, to stay in the same month.
The PR reverses that to normalize Feb 29 to March 1 to keep chronological order.

I'm not sure if switching the direction will have unintended consequence.

WDYT?

Or, is it possible for the PR to keep the current normalization?

Thanks for catching this edge case, @garydgregory!

Root Cause

The issue occurred because start.add(Calendar.YEAR, 1) clamped 2020-02-29 to 2021-02-28. Since the initial calculation for days had already subtracted 29 (1 - 29 = -28), adding the 28 days of February 2021 yielded -28 + 28 = 0 days instead of 1 day.

Solution

We don't need to change the normalization direction. Instead, we can unify the calculation with the rest of the token cascading hierarchy (y -> M -> d -> H -> m -> s -> S):

1. Apply standard hierarchical borrowing for days and months first (identical to when M is present).
2. If y is omitted, roll years into months (months += 12 * years; years = 0;).
3. If M is omitted, roll remaining months into days:

   if (!Token.containsTokenWithValue(tokens, M)) {
       while (months > 0) {
           days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
           months -= 1;
           start.add(Calendar.MONTH, 1);
       }
   }

@Alwaysgaurav1
Please rebate on git master to pick up the new test.

@garydgregory Rebased on master and updated the fix to unify the token cascading hierarchy.

Root Cause of the Test Failure

In the previous commit, start.add(Calendar.YEAR, 1) clamped 2020-02-29 to 2021-02-28. Because initial days subtracted 29 (1 - 29 = -28), adding the 28 days of February 2021 resulted in -28 + 28 = 0 days instead of 1 day.

Solution

We keep the exact same normalization behavior by unifying how omitted tokens are cascaded (y -> M -> d -> H -> m -> s -> S):

1. Apply standard hierarchical borrowing for days and months first (same as when M is present).
2. If y is omitted, roll years into months (months += 12 * years; years = 0;).
3. If M is omitted, roll remaining months into days:

   if (!Token.containsTokenWithValue(tokens, M)) {
       while (months > 0) {
           days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
           months -= 1;
           start.add(Calendar.MONTH, 1);
       }
   }

Thanks @Alwaysgaurav1 , merged 🚀